Skip to content

feat(config): trim the public surface and add a compiled build (CLI-2234, CLI-2232) - #6366

Open
Coly010 wants to merge 9 commits into
developfrom
columferry/cli-2234-audit-and-trim-supabaseconfigs-public-export-surface-before
Open

feat(config): trim the public surface and add a compiled build (CLI-2234, CLI-2232)#6366
Coly010 wants to merge 9 commits into
developfrom
columferry/cli-2234-audit-and-trim-supabaseconfigs-public-export-surface-before

Conversation

@Coly010

@Coly010 Coly010 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Closes CLI-2234
Closes CLI-2232

What changed

Prepares @supabase/config for its first npm publish: the export surface is audited and trimmed (CLI-2234) and the package gains a real compiled build (CLI-2232). One PR because the audit decides what the entrypoints contain and the build compiles exactly that, and the audit's type-surface enforcement (API report) only works once the build emits .d.ts.

Export-surface audit (CLI-2234)

Every export got an explicit keep / move / trim decision:

Decision Symbols
Moved to apps/cli KONG_LOCAL_CA_CERT (+ its test) — a local-stack TLS asset, not config schema
Moved to new ./internal subpath (explicitly not semver-covered; enforced apps/cli-only) ENV_CAPTURE_REGEX, AUTH_HOOK_NAMES, unmappedSecretApiPaths, projectConfigMappingRows, ProjectConfigMappingRow, ProjectConfigApiAttributes, InternalLoadCliConfigOptions + goViperCompat-capable typings of loadCliConfig/resolveCliConfigValue/resolveCliConfigSubtree
Removed from public options goViperCompat (off LoadCliConfigOptions; resolvers lost their options param entirely — re-adding later is non-breaking)
Trimmed MissingCliConfigValueError deleted entirely (never constructed anywhere in the package; apps/cli only carried a telemetry mapping entry + a test that fabricated one — both removed), loadCliConfigFile/InternalResolveCliConfigOptions off the internal barrel (zero consumers)
Renamed (./io, zero consumers existed) findCliProjectRootFor→findCliProjectRoot, findCliProjectPathsFor→findCliProjectPaths, loadCliProjectEnvironmentFor→loadCliProjectEnvironment, loadFunctionsManifest→inferFunctionsManifest./io now mirrors ./effect 1:1; the subpath conveys Promise-vs-Effect
Added sync resolveCliConfigValue/resolveCliConfigSubtree on . (Effect-typed variants deliberately shadow them on ./effect); ProjectConfigSchema (runtime, Standard Schema v1 via Schema.toStandardSchemaV1 — one symbol serves Effect-native and ~standard consumers); toProjectConfigJsonSchema; PROJECT_CONFIG_SCHEMA_URL
Kept (documented contract in README) schema/types, encoders, defaults + sparse helpers, the ProjectConfig converters (toProjectConfig, fromConfigDocument, fromApiProjectConfig, attachApiResponse, comparableProjectConfigPaths, unmappedApiFields), errors, functions-manifest model

ProjectConfigSchema is derived from CliConfigSchema at the AST level (hosted sections → type-side → deep-optional → x-secret leaves dropped → cross-field checks stripped, leaf checks kept) with a two-way compile-time assignability pin against the ProjectConfig type, so the runtime schema and the type cannot drift.

./io's error channel is narrowed from unknown to the exact five-member union (verified exact by review: no wider member, none unused).

Compiled build (CLI-2232)

  • Plain tsc (tsgo 7.0.2, nodenext + rewriteRelativeImportExtensions) emits ESM .js + .d.ts + maps to dist/; no bundler.
  • Conditional exports: in-repo Bun resolves src/*.ts (with customConditions: ["bun"] so tsc typechecks against source, not stale dist); external consumers get dist js + types.
  • Tarball sealed via files + .npmignore (npm-packlist otherwise applies the root .gitignore and ships no distnpm pack and pnpm pack now agree); publish metadata (license, repository, publishConfig, engines) added; peers widened to >=4.0.0-rc.111 <5.
  • sideEffects: false, proven by a tree-shake probe with positive + negative controls against the built artifact.
  • dist/project-schema.json joins dist/schema.json (both draft-2020-12, now with $id/title, and with Effect's non-finite-number anyOf encoding collapsed so numeric fields keep description/default).
  • The docs site now publishes those same built artifacts verbatim (generate-docs.ts copies dist/*.json to apps/docs/public/cli/{config,project-config}.schema.json; docs generate dependsOn the package build) — one post-processed source of truth, and both $id URLs resolve once deployed.
  • Workspace test runners resolve the bun export condition (resolve.conditions/ssr.resolve.conditions in the vitest configs), so vitest exercises src — never a stale or absent dist (caught by the AI review: previously tests resolved the defaultdist branch).
  • Build ends with a pack-and-install smoke test: real npm pack → temp install → real node imports every subpath.

Enforcement (surface changes stay deliberate)

  • Sealed exports map, pinned key set at runtime.
  • Per-entrypoint export-name snapshots (now incl. ./internal), purity walker over index.ts and io-browser.ts.
  • Type-surface changes: pnpm run check:config-api (root task; advisory continue-on-error CI step) emits declarations for the PR base and head — base source extracted via git archive into the package dir so the current install resolves deps, no second install — and reports the .d.ts diff in the job summary. Per-PR signal, zero committed artifacts. The hard gate moves to release time (CLI-2233): diff the new dist/*.d.ts against the previously published tarball's in the human-approval step.
  • @supabase/config/internal imports enforced apps/cli-only.

Review rounds

Three internal reviews (engineer, architect, DX-as-consumer incl. a clean-Node tarball install exercising 23 checks) ran before this PR; all accepted findings are in the final commit. Explicitly rejected, for the record:

  • @deprecated markers on ./internal exports (strikethrough noise across apps/cli's own legitimate call sites; the no-semver contract is documented at the barrel, README, and AGENTS.md).
  • message getters on the tagged error classes (would change CLI-visible error output pinned by normalize-error tests; README documents the structured-fields contract instead — candidate follow-up).
  • A checked-in api-report/ .d.ts mirror (53 files + freshness test) existed in earlier commits of this branch and was removed by owner decision — per-PR accept semantics weren't worth 580 KB of generated diff noise. Replaced by the advisory base-vs-head compare above; an api-extractor-style rollup was also considered and skipped (TS7/tsgo compatibility unproven).
  • saveCliConfig's atomic-write rename failure stays a defect (documented); re-channeling it as a typed failure is a behavioral follow-up.
  • Moving KONG_LOCAL_CA_CERT to packages/stack (single legacy consumer today; speculative second move).

Known collateral: @supabase/pg-topo under customConditions

customConditions: ["bun"] in apps/cli/tsconfig.json (needed so tsc typechecks @supabase/config against source instead of gitignored dist/) also changes resolution for @supabase/pg-topo, whose own bun exports condition points at unbuilt src/*.ts carrying 3 type errors at 1.0.0-alpha.5. Worked around with a commented paths pin to its shipped dist/index.d.ts. 1.0.0-alpha.6 is published but currently blocked by pnpm's minimumReleaseAge; once it ages in, bump it in apps/cli and drop the pin (and the bun-condition source errors deserve an upstream fix in supabase/pg-toolbelt either way).

Notes for CLI-2233 / CLI-2169 (publish)

  • The release pipeline must diff the new dist/*.d.ts against the previously published tarball's and surface that diff in the human-approval step — that is the hard semver gate (the PR-time compare above is advisory only; first publish trivially has no compare target).
  • Publish with pnpm publish (only pnpm rewrites any residual catalog:; peers are now literal ranges regardless).
  • effect@latest is still 3.x — README instructs effect@rc; revisit ranges when Effect 4 goes stable.
  • No top-level main/types (deliberate ESM + exports-only; node10 resolution unsupported).
  • Pre-existing @supabase/cli-go#lint:check gosec findings fail local check:all on clean develop too — untouched by this PR.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor
Superseded by a newer AI review

🤖 AI Review

This PR is too large for a full AI review (+10567/-520 lines across 118 files).

A maintainer can request a review anyway with a /ai-review comment.

1 similar comment
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor
Superseded by a newer AI review

🤖 AI Review

This PR is too large for a full AI review (+10567/-520 lines across 118 files).

A maintainer can request a review anyway with a /ai-review comment.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by a newer AI review

🤖 AI Review

Adjudicated 11 raw findings into 10 deduplicated findings: nine confirmed and one refuted. The most consequential issue is that unbuilt workspace tests can resolve @supabase/config to nonexistent dist files; the shallow-checkout API comparison and ProjectConfig primitive acceptance are also confirmed.

Findings

Severity Location Category Sources Claim
🟠 MAJOR packages/config/package.json:34 build-resolution claude Workspace tests can resolve ., ./effect, and ./internal to nonexistent dist files because only the bun condition points to source and Vitest is not configured to request that condition.
🟡 MINOR tools/config-api-compare.ts:100 ci-correctness claude+codex The API comparison fallback cannot reliably find a merge base from the workflow's depth-one checkout because it fetches only the base tip without deepening HEAD's history.
🟡 MINOR packages/config/src/internal.ts:21 api-contract claude InternalResolveCliConfigOptions is repeatedly documented as exported from @<!---->supabase/config/internal, but that entrypoint does not export it.
🟡 MINOR packages/config/src/schema-metadata.ts:3 schema-publishing claude PROJECT_CONFIG_SCHEMA_URL identifies a document that the docs generator never publishes, while the published CLI schema bypasses the post-processing applied to the packaged schema.
🟡 MINOR packages/config/src/project-config/project-schema.ts:151 validation codex Object-shaped fields that become empty AST object nodes accept primitive values, so ProjectConfigSchema can validate values such as db.vault: 42.
⚪ NIT packages/config/src/errors.ts:135 documentation claude Removing MissingCliConfigValueError leaves stale references to that symbol in the repository's root guidance documents.
⚪ NIT packages/config/scripts/json-schema-postprocess.ts:234 correctness claude withSchemaMetadata allows pre-existing root title and description values to overwrite the metadata supplied by its caller.
⚪ NIT packages/config/scripts/build.ts:86 path-handling claude JSON schema output paths are resolved from the process working directory rather than the package root used by the rest of the build.
⚪ NIT packages/config/AGENTS.md:120 documentation claude The testing documentation says three contract tests exist but lists four bullets containing five test modules.
Refuted findings (kept for transparency, not posted as review comments)
  • apps/cli/tsconfig.json:12 (documentation): The customConditions: ["bun"] change silently affects @supabase/stack and @supabase/api in addition to the specifically named pg-topo package.
    Refuted: The comment is deliberately general and explicitly warns that every other dependency declaring bun is affected; e.g. makes pg-topo non-exhaustive. Selecting the Bun variants is also consistent with this Bun-run CLI.

Stats

Claude findings: 9 · Codex findings: 2 · Confirmed: 9 · Refuted: 1 · Uncertain: 0


Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread packages/config/package.json
Comment thread tools/config-api-compare.ts
Comment thread packages/config/src/internal.ts
Comment thread packages/config/src/schema-metadata.ts
Comment thread packages/config/src/errors.ts
Comment thread packages/config/scripts/json-schema-postprocess.ts
Comment thread packages/config/scripts/build.ts Outdated
Comment thread packages/config/AGENTS.md Outdated
Comment thread packages/config/src/project-config/project-schema.ts
pull Bot pushed a commit to oogalieboogalie/cli that referenced this pull request Aug 28, 2026
…e on >=1 review (supabase#6380)

Follow-up to the AI-review pipeline. Two changes.

## 1. Downgrade `codex-action` v1.12 → v1.11

On the large PR (supabase#6366, ~130k-token diff) the `codex-review` step ran to
completion — Codex finished the turn, wrote its output file, printed its
final message and token count — then **sat idle until the 45-minute job
timeout**, discarding a completed review. That is a confirmed v1.12
regression: **openai/codex-action#150** ("v1.12: Linux run never returns
after the turn completes; job dies on timeout with the output file
already written"). The reporter confirms **v1.11 handles the same heavy
workload cleanly**, and there is no released fix above v1.12.

v1.11 (`52fe01ec…`) supports every input we use (`safety-strategy`,
`sandbox`, `output-schema-file`, `output-file`, `codex-version`,
`working-directory`, `effort`), so this is a drop-in pin change in both
Codex jobs.

## 2. Adjudicate on ≥1 independent review (graceful degradation)

Previously `adjudicate` required BOTH `claude-review` and `codex-review`
to succeed, so one flaky model job sank the whole review. Now it runs
when **at least one** independent pass succeeded: each findings download
is guarded by its job's result, and the stage step substitutes an empty
findings set for any review that didn't complete, so the adjudicator
reconciles one or two. The prompt notes the one-review case and records
it in its summary.

Together: a Codex hiccup no longer wastes a 45-minute run or blocks
Claude's (working) review from being posted.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by a newer AI review

🤖 AI Review

Two documentation findings are confirmed. The packaging finding is refuted because the README explicitly says publication is deferred, while Claude's placeholder finding contains no verifiable claim and remains uncertain.

Findings

Severity Location Category Sources Claim
🟡 MINOR apps/cli/vitest.config.ts:25 testing claude The submitted finding only claims "test" and does not identify a concrete defect.
🟡 MINOR packages/config/README.md:284 documentation codex The documented ./io error-handling contract gives contradictory and unusable type/message guidance.
⚪ NIT packages/config/README.md:265 documentation codex The README incorrectly calls the two resolver functions the only non-Promise exports from ./io.
Refuted findings (kept for transparency, not posted as review comments)
  • packages/config/package.json:4 (packaging): The package remains marked private, so npm will refuse to publish the newly prepared public package.
    Refuted: The private flag is intentional for this PR. The newly added README explicitly documents deferred publication, and trusted/CLAUDE.md says internal TypeScript workspaces such as packages/config generally follow the private workspace template.

Stats

Claude findings: 1 · Codex findings: 3 · Confirmed: 2 · Refuted: 1 · Uncertain: 1


Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread apps/cli/vitest.config.ts
Comment thread packages/config/README.md Outdated
Comment thread packages/config/README.md Outdated
@Coly010 Coly010 self-assigned this Aug 30, 2026
@Coly010
Coly010 marked this pull request as ready for review August 30, 2026 13:31
@Coly010
Coly010 requested a review from a team as a code owner August 30, 2026 13:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-30T13:36:20.427211Z 9872825 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@98728256a07c47f5fba0710803d20106163759a0

Preview package for commit 9872825.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant